feat(sqllab): Improved query status indicator bar#36936
feat(sqllab): Improved query status indicator bar#36936justinpark merged 11 commits intoapache:masterfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #ebadebActionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
justinpark
requested review from
michael-s-molina,
mistercrunch and
rusackas
and removed request for
rusackas
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| ); | ||
| const prevStepRef = useRef<number>(0); | ||
| const progress = | ||
| query.progress > 0 ? parseInt(query.progress.toFixed(0), 10) : undefined; |
There was a problem hiding this comment.
Suggestion: Calls toFixed on query.progress without ensuring it's a number; if query.progress is undefined this will throw. Guard the access and use a safe rounding method when query.progress is a number. [type error]
Severity Level: Minor
| query.progress > 0 ? parseInt(query.progress.toFixed(0), 10) : undefined; | |
| typeof query.progress === 'number' && query.progress > 0 | |
| ? Math.round(query.progress) | |
| : undefined; |
Why it matters? ⭐
If query.progress can be undefined (or a non-number), calling toFixed could throw or generate TypeScript errors. Guarding with a typeof check and using Math.round is clearer and safer for both runtime and type-checking. The current conditional (query.progress > 0) prevents the true-branch from executing when progress is undefined at runtime, but an explicit typeof check is more robust and explicit for maintainers and the type system.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryStatusBar/index.tsx
**Line:** 157:157
**Comment:**
*Type Error: Calls `toFixed` on `query.progress` without ensuring it's a number; if `query.progress` is undefined this will throw. Guard the access and use a safe rounding method when `query.progress` is a number.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| const prevStepRef = useRef<number>(0); | ||
| const progress = | ||
| query.progress > 0 ? parseInt(query.progress.toFixed(0), 10) : undefined; | ||
| const { progress_text: progressText } = query.extra; |
There was a problem hiding this comment.
Suggestion: Destructuring query.extra without checking it can throw if extra is undefined; access progress_text via optional chaining and provide a safe default instead of directly destructuring from query.extra. [null pointer]
Severity Level: Minor
| const { progress_text: progressText } = query.extra; | |
| const progressText = query.extra?.progress_text ?? ''; |
Why it matters? ⭐
The current code destructures query.extra directly which will throw if query.extra is ever undefined at runtime.
Switching to optional chaining is a small, safe defensive change that prevents a potential crash and matches how the value is later used with a fallback (progressText ?? ''). This also avoids TypeScript complaints if extra is typed as optional.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryStatusBar/index.tsx
**Line:** 158:158
**Comment:**
*Null Pointer: Destructuring `query.extra` without checking it can throw if `extra` is undefined; access `progress_text` via optional chaining and provide a safe default instead of directly destructuring from `query.extra`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
💡 Enhance Your PR ReviewsWe noticed that 3 feature(s) are not configured for this repository. Enabling these features can help improve your code quality and workflow: 🚦 Quality GatesStatus: Quality Gates are not enabled at the organization level 🎫 Jira Ticket ComplianceStatus: Jira credentials file not found. Please configure Jira integration in your settings ⚙️ Custom RulesStatus: No custom rules configured. Add rules via organization settings or .codeant/review.json in your repository Want to enable these features? Contact your organization admin or check our documentation for setup instructions. |
Code Review Agent Run #66dee5Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #7fcee6Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #51a851Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
cc: @kasiazjc for design feedback |
justinpark
force-pushed
the
feat--improved-query-status-progress
branch
from
5a509e4 to
e6e3784
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
superset-frontend/src/SqlLab/components/QueryStatusBar/QueryStatusBar.test.tsx
Show resolved
Hide resolved
|
CodeAnt AI Incremental review completed. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #0ff81a
Actionable Suggestions - 2
-
superset-frontend/packages/superset-ui-core/src/components/Timer/index.tsx - 1
- Logic Inconsistency in Timer Init · Line 38-40
-
superset-frontend/src/SqlLab/actions/sqlLab.ts - 1
- Incorrect sync query state · Line 394-396
Additional Suggestions - 1
-
superset-frontend/src/SqlLab/components/ResultSet/index.tsx - 1
-
Removed UI feedback elements · Line 39-39The removal of Loading, ProgressBar, and QueryStateLabel imports and their usage in the render logic eliminates visual feedback that was previously shown during query execution (e.g., loading spinner when progress <= 0, progress bar when > 0, and state label always). This changes observable program behavior by potentially making the UI appear unresponsive during long-running queries, as users now only see the progressMsg Alert if provided by the backend. If the progressMsg is intended as the sole feedback mechanism, this may be acceptable, but it risks degrading user experience without clear replacement.
-
Review Details
-
Files reviewed - 8 · Commit Range:
d0ab782..e6e3784- superset-frontend/packages/superset-ui-core/src/components/Timer/index.tsx
- superset-frontend/packages/superset-ui-core/src/query/types/Query.ts
- superset-frontend/src/SqlLab/actions/sqlLab.ts
- superset-frontend/src/SqlLab/components/QueryStatusBar/QueryStatusBar.test.tsx
- superset-frontend/src/SqlLab/components/QueryStatusBar/index.tsx
- superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
- superset-frontend/src/SqlLab/components/ResultSet/index.tsx
- superset-frontend/src/SqlLab/components/SouthPane/Results.tsx
-
Files skipped - 0
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
superset-frontend/packages/superset-ui-core/src/components/Timer/index.tsx
Show resolved
Hide resolved
justinpark
force-pushed
the
feat--improved-query-status-progress
branch
from
e6e3784 to
58b58a2
Compare
Code Review Agent Run #abe235Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
2 participants
SUMMARY
The existing query status indicator makes it difficult to predict the progress, so this commit introduces a new progress status that shows which stage is currently in progress. This change will allow users to more accurately anticipate the query execution process and wait accordingly.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--query-state.mov
After:
improved_query_state.mov
progress_query_state.mov
failed_query.mov
TESTING INSTRUCTIONS
specs are added
ADDITIONAL INFORMATION